Use saturating_mul when multiplying feerates by the fee spike buf#4554
Conversation
|
I've assigned @wpaulino as a reviewer! |
TheBlueMatt
force-pushed
the
2026-04-feerate-overflows
branch
from
76d1774 to
a40e360
Compare
|
I've reviewed the full diff and surrounding context. The three No new issues found. |
In theory a channel's feerate could be set to some absurd value (millions of satoshis per vB) and we'd overflow the fee spike buffer, accepting the absurd fee and ignoring our fee spike buffer check. This is harmless - the counterparty has much easier ways of bricking the channel if they want, and paying several BTC in fees is probably not the best way. Our commitment transaction and dust fee exposure logic all correctly map the `u32` to a `u64` before multiplying, making them overflow-safe. Still, its good to fix overflows because it is a remotely-reachable crash in debug builds. Reported by Jordan Mecom of Block's Security Team
TheBlueMatt
force-pushed
the
2026-04-feerate-overflows
branch
from
a40e360 to
b98d7b8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4554 +/- ##
==========================================
- Coverage 87.01% 86.97% -0.05%
==========================================
Files 163 163
Lines 108682 108683 +1
Branches 108682 108683 +1
==========================================
- Hits 94572 94524 -48
- Misses 11631 11678 +47
- Partials 2479 2481 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Looking forward to get rid of the entire class of bug by migrating to FeeRate in v0.4. :)
4 participants
In theory a channel's feerate could be set to some absurd value (millions of satoshis per vB) and we'd overflow the fee spike buffer, accepting the absurd fee and ignoring our fee spike buffer check. This is harmless - the counterparty has much easier ways of bricking the channel if they want, and paying several BTC in fees is probably not the best way. Our commitment transaction and dust fee exposure logic all correctly map the
u32to au64before multiplying, making them overflow-safe.Still, its good to fix overflows because it is a remotely-reachable crash in debug builds.
Reported by Jordan Mecom of Block's Security Team